Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better explain that form types should be unique in the application #5076

Merged
merged 5 commits into from
Mar 24, 2015

Conversation

javiereguiluz
Copy link
Member

Q A
Doc fix? yes
New docs? no
Applies to all
Fixed tickets #4960

The ``getName()`` method returns the identifier of this form "type". These
identifiers must be unique in the application and, unless you want to override
a built-in type, they should be different from the default Symfony types.
Consider prefixing your types with ``app_`` to avoid identifier collisions.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this become a best practice ?

Consider prefixing your types with ``app_`` to avoid identifier collisions.

This new class contains all the directions needed to create the task form. It can
be used to quickly build a form object in the controller:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to colons here. Otherwise, the following code block won't be recognized.

@javiereguiluz
Copy link
Member Author

Thanks for your review. Everything has been fixed/updated and I've created a separate issue (#5077) to discuss about introducing a new best practice for Symfony forms.

identifiers must be unique in the application. Unless you want to override
a built-in type, they should be different from the default Symfony types
and from any type defined by a third-party bundle installed in you application.
Consider prefixing your types with ``app_`` to avoid identifier collisions.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the best practices, we use app. (dot instead of underscore) for service names. Should we follow this convention here too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you. I've just updated the PR with your suggestion.

identifiers must be unique in the application. Unless you want to override
a built-in type, they should be different from the default Symfony types
and from any type defined by a third-party bundle installed in you application.
Consider prefixing your types with ``app.`` to avoid identifier collisions.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, Javier, we also need to switch back to app_ here as @stof explained in #5077.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. It's done in 0eb149b. Thanks.

The ``getName()`` method returns the identifier of this form "type". These
identifiers must be unique in the application. Unless you want to override
a built-in type, they should be different from the default Symfony types
and from any type defined by a third-party bundle installed in you application.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

your

@weaverryan
Copy link
Member

Thanks Javier! I don't like needing this warning, but it makes sense. It does beg the question though - should we prefix all of these with app_ in the docs?

@weaverryan weaverryan merged commit 7a78532 into symfony:2.3 Mar 24, 2015
weaverryan added a commit that referenced this pull request Mar 24, 2015
…plication (javiereguiluz)

This PR was merged into the 2.3 branch.

Discussion
----------

Better explain that form types should be unique in the application

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | all
| Fixed tickets | #4960

Commits
-------

7a78532 Fixed a minor grammar issue
0eb149b Recommend app_ instead of app. as form type prefix
52a5551 Recommend a better way to standardize application form types
d47e751 Fixed syntax issues and provided more information
12b77af Better explain that form types should be unique in the application
@weaverryan
Copy link
Member

Ha, yes I see that's exactly what you're asking in #5077 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants